Skip to content

bpo-39681: Fix C pickle regression with minimal file-like objects#18592

Merged
ambv merged 1 commit into
python:masterfrom
pitrou:bpo-39681-pickle-readinto
Feb 23, 2020
Merged

bpo-39681: Fix C pickle regression with minimal file-like objects#18592
ambv merged 1 commit into
python:masterfrom
pitrou:bpo-39681-pickle-readinto

Conversation

@pitrou

@pitrou pitrou commented Feb 21, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.

https://bugs.python.org/issue39681

@pitrou
bpo-39681: Fix C pickle regression with minimal file-like objects
7a3d53c 
Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.
@pitrou pitrou requested a review from ambv February 21, 2020 13:26
@pitrou

pitrou commented Feb 21, 2020

Copy link
Copy Markdown
Member Author

cc @pierreglaser and @ogrisel if you want to give this a quick look.

@codecov

codecov Bot commented Feb 21, 2020

Copy link
Copy Markdown

Codecov Report

Merging #18592 into master will increase coverage by 1.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #18592       +/-   ##
===========================================
+ Coverage   82.11%   83.13%    +1.01%     
===========================================
  Files        1956     1571      -385     
  Lines      589402   414817   -174585     
  Branches    44458    44458               
===========================================
- Hits       484012   344841   -139171     
+ Misses      95739    60348    -35391     
+ Partials     9651     9628       -23
Impacted Files Coverage Δ
Lib/distutils/tests/test_bdist_rpm.py 30.00% <0.00%> (-65.00%) ⬇️
Lib/distutils/command/bdist_rpm.py 7.63% <0.00%> (-56.88%) ⬇️
Lib/test/test_urllib2net.py 76.92% <0.00%> (-13.85%) ⬇️
Lib/test/test_smtpnet.py 78.57% <0.00%> (-7.15%) ⬇️
Lib/ftplib.py 63.85% <0.00%> (-6.06%) ⬇️
Lib/test/test_ftplib.py 87.11% <0.00%> (-4.72%) ⬇️
Tools/scripts/db2pickle.py 17.82% <0.00%> (-3.97%) ⬇️
Tools/scripts/pickle2db.py 16.98% <0.00%> (-3.78%) ⬇️
Lib/test/test_socket.py 71.94% <0.00%> (-3.77%) ⬇️
Lib/test/test_asyncio/test_base_events.py 91.84% <0.00%> (-3.30%) ⬇️
... and 436 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4d17fd...7a3d53c. Read the comment docs.

pitrou
pitrou commented Feb 21, 2020
View reviewed changes
Comment thread Modules/_pickle.c

if (!self->readinto) {
/* readinto() not supported on file-like object, fall back to read()
* and copy into destination buffer (bpo-39681) */

@pitrou pitrou Feb 21, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should deprecate this fallback path in 3.9?

@ambv ambv merged commit 9f37872 into python:master Feb 23, 2020
@bedevere-bot

bedevere-bot commented Feb 23, 2020

Copy link
Copy Markdown

@ambv: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington

miss-islington commented Feb 23, 2020

Copy link
Copy Markdown
Contributor

Thanks @pitrou for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot

bedevere-bot commented Feb 23, 2020

Copy link
Copy Markdown

GH-18630 is a backport of this pull request to the 3.8 branch.

@pitrou pitrou deleted the bpo-39681-pickle-readinto branch February 23, 2020 22:44
ambv pushed a commit that referenced this pull request Feb 23, 2020
@miss-islington @pitrou
bpo-39681: Fix C pickle regression with minimal file-like objects (GH…
b19f7ec 
…-18592) (#18630)

Fix a regression where the C pickle module wouldn't allow unpickling from a
file-like object that doesn't expose a readinto() method.
(cherry picked from commit 9f37872)

Co-authored-by: Antoine Pitrou <antoine@python.org>

Co-authored-by: Antoine Pitrou <pitrou@free.fr>
@ogrisel

ogrisel commented Feb 24, 2020

Copy link
Copy Markdown
Contributor

LGTM. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ambv ambv Awaiting requested review from ambv

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pitrou @bedevere-bot @miss-islington @ogrisel @ambv @the-knights-who-say-ni